As this affects all networks and is a conversation starter at this early stage, I did not do comprehensive tests to confirm that everything works as intended.

When we revamp the integration tests for the generative networks in the near future, it should be no problem to cover usage with and without sample weights.

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 78.26087% with 5 lines in your changes missing coverage. Please review.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

... and 11 files with indirect coverage changes

True, sample weights are also supported by keras already, but I think we still have to do some changes to fully support them.

As far as I can tell, the proposed changes are not reimplementing anything that keras.Metrics already provides, because it concerns how sample weights get propagated through the approximator, including to any keras.Metrics that should be weighted.

When fitting with a PyDataset, keras accepts sample weights as a third tuple item (x,y,sample_weight).
So it is my impression that adding sample_weight next to inference_variable, inference_condition and summary condition is in line with both the keras and the bayesflow way.

We should mimic keras behavior regarding metrics insofar as to only pass weights to metrics in the self.weighted_metrics attribute. For reference: https://keras.io/api/models/model_training_apis/#compile-method

@LarsKue, do you have a direction in mind to further exploit the preexisting keras implementation of sample_weight?

@han-ol Thank you for clarifying. I think I see the idea behind the changes now.

Since you mentioned that the changes are only a conversation starter for now, could you illustrate some use cases (w.r.t. user code) and perhaps also let us know when you think these changes are production-ready?

@stefanradev93 What do you think about the current state of this PR?

I think it's a nice to have option, especially in terms of potential future adaptations.

I made some experiments with weighting already, so I can definitely show it in use if you are interested. However, I personally cannot provide an example where weighting gives a tangible benefit over other approaches yet.

Having the infrastructure already and agreeing on UI here sooner rather than later is good, IMO, because this is straightforward to include now, and can then find its way into the docs, tests, etc. at a time when we are (re)writing old and new docs anyways.

Abstractly speaking, this sort of sample weights is useful

• whenever we have a simulator that generates samples from a proposal distribution and importance weights wrt a target distribution,
• for weighted scoring rules (when you care mostly about some part of parameter space)

Regarding timeline:
If there is no objection to the general implementation approach, we can merge this in the next days. Prior to merging I would like to have

• approval of the people who implemented each of the changed generative network loss functions
• your opinion on how to aggregate: the PR currently adds to each inference network its own aggregate() method (just once due to inheritance) that accepts losses and optional weights, then reduces with keras.ops.mean(). On the one hand, this is nice because it let's users overwrite just the aggregate method if they have special sample_weights. On the other hand, it is simpler to have one aggregate function in bayesflow/utils. I would probably change it to the second (simpler) solution here.

Another todo for the functionality is

• tests

Please let me know if I should add tests prior to merging, or if it makes more sense to add tests during or after refactor of the pre-existing network and integration tests.

I can confirm these are highly relevant usecases for weighting and we will need them sooner rather than later. So I would be very happy to see this PR being merged soon!

@LarsKue @paul-buerkner Should we integrate this functionality?

Ok, let's merge dev into this branch and I will accept the merge.

Merging done. Quick question: Is "sample_weights" the best name for it? We could also just use "weights" right? Or would this be ambigous?

@paul-buerkner I think both would be fine. sample_weight (without the s) is what keras calls it.

Let's use singular then. Because sample_weight will always be only a single variable, right (data dim = 1)? Not multiple variables as is the case for inference_variables etc. @han-ol if you agree, would you mind changing that?

Yes, agreed.

I changed the names "sample_weights" -> "sample_weight" everywhere and also changed the default adapter to expect just one string that is renamed to "sample_weight" (rather than a Sequence of multiple, that is concatenated into "sample_weights").

I added tests as well and support for point inference networks, so the PR is ready to undergo review at this point.